There was really a lot of work done only to be able to call web-stream-tools readToEnd function, with different approaches for unit test code, extension pages and content script. May it be possible to convince OpenPGP.js developers to simply re-export this helper function from their library, @tomholub ?

The main problem with Firefox was that Uint8Array's created by OpenPGP.js were of globalThis.Uint8Array or window.Uint8Array types
and this fix openpgpjs/web-stream-tools#23 didn't help because
calling a method of such an object (e.g. stripped = bin.subarray(i); inside OpenPGP.js) caused an exception Error: Permission denied to access property "constructor"
However, this type of workaround works: new Uint8Array(bin).subarray(i)
So this patch iterates over the tree of OpenPGP.Key object to re-initialize all Uint8Array's with new Uint8Array

Opinions, @sosnovsky, @tomholub?

I imagine this could be fixed purely in OpenPGP.js which would be cleaner? Either that, or monkey-patch the types on our end.

The main problem with Firefox was that Uint8Array's created by OpenPGP.js were of globalThis.Uint8Array or window.Uint8Array types

Does that mean they create sometimes one type and sometimes the other?

Option 1: Can the library be patched to always create the same "good" type?

Or are you saying that both globalThis.Uint8Array and window.Uint8Array are "bad" and we need just Uint8Array? Where does that one come from if not global or window?

Option 2: Could the library be patched to accept a config.Uint8ArrayClass = Uint8Array and then that would be used throughout the codebase? That would be a larger change, presumably.

Option 3: Alternatively, on the "broken" type, could we monkey patch it from outside the library like this? Right in requireOpenpgpjs

globalThis.Uint8Array.prototype.subarray = function (i) {
  return new Uint8Array(bin).subarray(i);
}

window.Uint8Array.prototype.subarray = function (i) {
  return new Uint8Array(bin).subarray(i);
}

The main problem with Firefox was that Uint8Array's created by OpenPGP.js were of globalThis.Uint8Array or window.Uint8Array types

Does that mean they create sometimes one type and sometimes the other?

1. With regard to this particular test if (x instanceof window.Uint8Array) seems to be good enough, while if (x instanceof globalThis.Uint8Array) causes the test to fail.
2. The window.openpgp (where we take the instance of OpenPGP.js) was undefined when loading the OpenPGP.js bundle in Firefox, so I changed it to globalThis.openpgp. This works in both Chrome and Firefox. So globalThis in Firefox is a sandbox, while in Chrome it's a window.
3. And the issue (that I dealt with earlier) https://github.com/openpgpjs/web-stream-tools/pull/23/files is that OpenPGP.js doesn't recognize its "own" Uint8Array type when running in a Firefox sandbox.

I just had another look at OpenPGP.js code (and there is native Web Cryptography API involved) and have this hypothesis (not tested yet):
As the "Permission denied" error was encountered in serializeParams function, probably these "bad" window.Uint8Array objects are returned from this Web Cryptography API, while globalThis.Uint8Array (sandboxed) my be created by the OpenPGP.js code itself. And the "constructor" permission denied error actually means that subarray is trying to create window.Uint8Array (same as itself) but as it is called by a sandbox only globalThis.Uint8Array are allowed to be created, hence the cul-de-sac. We can try and file this as a bug to Firefox.

Option 1: Can the library be patched to always create the same "good" type?

The code itself only has simple new Uint8Array calls, so the fact that it creates globalThis.Uint8Array is a consequence that the code is running in a sandbox. So this question should be rephrased as -- can and should we make the code run not in a sandbox in Firefox? If not, we can probably try and update the library to re-initialize all Uint8Array's coming from Web Cryptography API (that is, convert window.Uint8Array to sandboxed globalThis.Uint8Array), but how to unit-test this? We need to handle each place where Uint8Array arrives from the API and make sure we handle each new such place.

Or are you saying that both globalThis.Uint8Array and window.Uint8Array are "bad" and we need just Uint8Array? Where does that one come from if not global or window?

Apparently this is due to the complex Firefox process model (that I don't fully understand yet) https://wiki.mozilla.org/Security/Sandbox/Process_model
Looks like globalThis.Uint8Array (sandbox) is bearable in a sense that it can be dealt with patches like https://github.com/openpgpjs/web-stream-tools/pull/23/files but there may be problems in the code consuming these objects should they try to test their types.

Option 2: Could the library be patched to accept a config.Uint8ArrayClass = Uint8Array and then that would be used throughout the codebase? That would be a larger change, presumably.

It's not an issue of mis-configuration.

Option 3: Alternatively, on the "broken" type, could we monkey patch it from outside the library like this? Right in requireOpenpgpjs

globalThis.Uint8Array.prototype.subarray = function (i) {
  return new Uint8Array(bin).subarray(i);
}

window.Uint8Array.prototype.subarray = function (i) {
  return new Uint8Array(bin).subarray(i);
}

I can try this.

This works in both Chrome and Firefox:

    if (window !== globalThis) {
        window.Uint8Array.prototype.subarray = function (i) {
            return new globalThis.Uint8Array(this).subarray(i);
        };
    }

and this too:

    if (window !== globalThis) {
        window.Uint8Array.prototype.subarray = function (i) {
            return new Uint8Array(this).subarray(i);
        };
    }

we can also add Catch.browser().name === 'firefox' into the condition.

The following results in an infinite recursion in Chrome:

        window.Uint8Array.prototype.subarray = function (i) {
            return new Uint8Array(this).subarray(i);
        };

But may this patch possibly affect other scripts on the page? Is window of the content script isolated from the window of the page itself?

we can also add Catch.browser().name === 'firefox' into the condition.

yes. This looks good. If that's the case, we don't have to test Firefox so thoroughly, since it's a global fix, and we also wouldn't have to set up Firefox automated tests at this time.

we don't have to test Firefox so thoroughly

There potentially might be issues with other objects than Uint8Array. Also we might need to patch other methods that return an array, e.g. slice and filter?

I tested slice and filter -- slice failed so I patched it too, filter is working without modification.

I also tested the "bad" Uint8Array as input to a function like readBinary() or openpgp.readKey{ binaryKey: ... }
When unpatched, the permission denied exception happens.
When patched everything is ok.
My concern is whether our prototype patch applies to all potential sources of Uint8Arrays such as network, background page etc.?
Should we pre-emptively re-initialize all Uint8Array arguments of, say, KeyUtil and MsgUtil methods?

Should we pre-emptively re-initialize all Uint8Array arguments of, say, KeyUtil and MsgUtil methods?

I wouldn't. I'd just go with the patches that appear to work, do some manual testing on Firefox, release it, wait for any reports and then fix those (which should be coming from less common workflows).

We only probably have 5k users on Firefox, as opposed to hundreds of thousands on Chrome. Plus I treat Firefox as our beta channel. So I'm not too worried, and wouldn't want to over-focus on this.

With this build firefox-consumer-content-script-tests I'm able to manually run the test in Firefox and see the result like this (the script is for some unknown reason called 3 times in Firefox. while only once in Chrome) when opening the gmail page

Should we look into automating firefox tests?

The build chrome-consumer-content-script-tests-mock is used in automated testing

The test content script is called 3 times in Firefox for some unknown reason (while only once in Chrome) when opening the gmail page

This is an example of success

Maybe they run 3 times as a retry because they are failing?

Because of these 2 exceptions?

1. Changes in this file are ignored on incremental builds for some reason.
2. I think it makes sense to test all possible OpenPGP.js use cases from the content script in Firefox

This basically wipes out the webMail code, only libraries are left and test/source/tests/content-script-test.js
immediately calls OpenPGP.js and web-stream-tools directly

Also BrowserMsg.addPgpListeners(); could be removed from below, and added to content script message listener instead.

Then pgp_block can be updated to call content script instead of background script.

Should I do this in this PR?
I thought after #5022 we may no longer need some of these messages, e.g. pgpMsgDecrypt or pgpMsgVerifyDetached

i see. then it doesn't have to be in this pr.